Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] Swallow ObjectDisposedException when aborting QuicStream from CancellationAction #75179

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 7, 2022

Backport of #74634 to release/7.0
Fixes #73688

This change depends on #75192 (which has been already merged into release/7.0).

/cc @rzikm

Customer Impact

In certain situations, aborting QuicStream operations using CancellationTokenSource.CancelAfter may cause an exception to be thrown inside ValueTask continuation processing code. The end result is crashing the process on unhandled exception. This PR makes sure we catch and swallow the exception.

The exception is not a symptom of a deeper problem, since #75192, the race which causes the exception is harmless.

Discovered in HTTP/3 stress runs and also in CI.

Testing

CI functional tests are passing, also verified manually with local changes to make the race more frequent.

Risk

Low, System.Net.Quic is still in preview.

@ghost
Copy link

ghost commented Sep 7, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #74634 to release/7.0

/cc @rzikm

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net

Milestone: -

@rzikm rzikm self-assigned this Sep 7, 2022
@karelz karelz added the blocked Issue/PR is blocked on something - see comments label Sep 7, 2022
@karelz karelz added this to the 7.0.0 milestone Sep 7, 2022
@carlossanlop carlossanlop added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 7, 2022
@danmoseley
Copy link
Member

approved. stability of new feature area.

@karelz
Copy link
Member

karelz commented Sep 8, 2022

The PR this depends on (#75192) has been merged into release/7.0. removing the no-merge and blocked labels.

@karelz karelz removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) blocked Issue/PR is blocked on something - see comments labels Sep 8, 2022
@carlossanlop
Copy link
Member

carlossanlop commented Sep 8, 2022

Approved, signed off, CI is green (failures seem unrelated), and it's unblocked now. Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 5d71eeb into release/7.0 Sep 8, 2022
@carlossanlop carlossanlop deleted the backport/pr-74634-to-release/7.0 branch September 8, 2022 17:18
@ghost ghost locked as resolved and limited conversation to collaborators Oct 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants